-
-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: lint and pre-commit hook with Husky and eslint #33
feat: lint and pre-commit hook with Husky and eslint #33
Conversation
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected], npm/[email protected] |
a33e647
to
7de03f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested the changes yet. Will pull down and test the PR a little later today 🙂
Our prop types are fine, but eslint-react isn't detecting them correctly. With a little help jsx-eslint/eslint-plugin-react#3284 (comment) we can tell it what the props are.
Same principle as the main repo: if we care enough for something to be a rule, we want CI to tell us when it's not being followed.
@huyenltnguyen I had to fix a few errors, but nothing that required me to change the functionality, just types. I hope that's okay! If I messed anything up, please let me know/push a commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together, @Sembauke.
I made a few tweaks, so I'll explain the justification:
- Similarly to the main repo, treat eslint warnings as errors (otherwise they're likely to be ignored)
- Use recommended rules for all plugins. They're generally good defaults, so we should use them until we have a reason not to
- Add
eslint .
tolint
script. Otherwise CI will not run it.
Checklist:
Update index.md
)Closes #XXXXX